Conversation
504110d to
1a13965
Compare
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
1a13965 to
b8e02c0
Compare
bdc4c89 to
85c60f8
Compare
85c60f8 to
56f5732
Compare
fcdd1f3 to
cc26d68
Compare
daa5828 to
6cd04a3
Compare
Contributor
Author
|
We could remove the |
6cd04a3 to
a3838dc
Compare
Contributor
Author
PR History: #763 · build: Bundle vats with vite
Jan 22 · rekmarks · CHANGES_REQUESTED — 5/5 ●
Jan 22 · cursor · COMMENTED — 2/2 ●
Jan 23 · cursor · COMMENTED — 4/4 ●
Jan 23 · cursor · COMMENTED — 2/2 ●
Jan 23 · cursor · COMMENTED — 1/1 ●
|
rekmarks
reviewed
Jan 29, 2026
Comment on lines
22
to
31
| export const isVatBundle = (value: unknown): value is VatBundle => | ||
| isObject(value) && | ||
| hasProperty(value, 'moduleFormat') && | ||
| value.moduleFormat === 'iife' && | ||
| hasProperty(value, 'code') && | ||
| typeof value.code === 'string' && | ||
| hasProperty(value, 'exports') && | ||
| Array.isArray(value.exports) && | ||
| hasProperty(value, 'external') && | ||
| Array.isArray(value.external); |
Member
There was a problem hiding this comment.
This should be done with superstruct
packages/cli/src/vite/vat-bundler.ts
Outdated
Comment on lines
36
to
37
| stripCommentsPlugin() as unknown as PluginOption, | ||
| metadataPlugin as unknown as PluginOption, |
Member
There was a problem hiding this comment.
Odd that we have to cast them here.
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
Reuses the acorn parsing dependency from `@ocap/kernel-agents-repl` to authoritatively scrub comments from vat bundles. Refs #770 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Replaces the comment scrubber with an AST-based implementation to reliably remove comments (including those containing `import(`) from bundled code. > > - Refactors `vite/strip-comments-plugin` to use Acorn (`parse` with `onComment`) and return unchanged code when no comments are found > - Adds unit tests for `strip-comments-plugin` covering single/multi-line comments, strings, regex, templates, and empty input > - Adds `acorn` dependency in `@ocap/cli` and updates `yarn.lock` > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3a59ba8. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
status: substantiated ### Incomplete type guard doesn't validate all VatBundle properties **Medium Severity** The `isVatBundle` type guard asserts that a value is a `VatBundle`, but only validates `moduleFormat` and `code` properties. The `VatBundle` type also requires `exports: string[]` and `modules: Record<...>` properties. Code that uses this type guard will believe it has a complete `VatBundle` after the check, but accessing `.exports` or `.modules` could return `undefined` if an incomplete object passed the check. Location: packages/kernel-utils/src/vat-bundle.ts#L24-L30
status: substantiated ### Missing validation of `code` property in bundle loader **Medium Severity** The `loadBundle` function validates that `moduleFormat` equals `'vite-iife'` but does not validate that `code` exists or is a string before using it in `compartment.evaluate`. If a malformed bundle is loaded (e.g., corrupted file, incompatible version) with correct `moduleFormat` but missing or non-string `code`, the template literal interpolation would produce invalid JavaScript (like `"undefined"` or `"[object Object]"`) leading to confusing evaluation errors or silently returning `undefined` instead of proper exports. Location: packages/ocap-kernel/src/vats/bundle-loader.ts#L29-L33
status: substantiated ### Wrong sourceType for parsing IIFE bundle output **Low Severity** The `stripCommentsPlugin` uses `sourceType: 'module'` to parse vite's IIFE output, but IIFE bundles are scripts, not modules. Module parsing implies strict mode, which rejects certain legacy JavaScript patterns (octal literals, `with` statements, etc.). If bundled dependencies contain non-strict-mode code that wasn't transpiled, `parse()` throws a SyntaxError, preventing comment stripping and causing the build to fail unexpectedly. Location: packages/cli/src/vite/strip-comments-plugin.ts#L21-L26
Add validation that code exists and is a string before calling compartment.evaluate in loadBundle. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add validation for exports (Array.isArray) and modules (object && !== null) properties in the isVatBundle type guard. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change sourceType from 'module' to 'script' in stripCommentsPlugin to correctly handle IIFE bundles that may contain non-strict-mode patterns like octal literals and with statements. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove `export` keyword from BundleMetadata type (only used internally) - Replace modules metadata collection with simple `external: []` field - Update VatBundle type and isVatBundle type guard accordingly - Update vat-bundle tests to use external instead of modules Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update the test bundle fixture to use `external: []` instead of
`modules: {}` to match the updated VatBundle type definition.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use parsed.code directly after validation instead of casting to an intermediate VatBundle type that is only used for its code property. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Address rekmarks review comments: - Use superstruct for VatBundle type guard instead of manual checks - Import Plugin from vite instead of rollup to remove unnecessary casts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
d511472 to
c0cf139
Compare
Tests that loadBundle throws clear validation error for malformed content. Currently fails with TypeError instead of descriptive error message. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Validate that parsed JSON is a non-null object before accessing properties. Prevents TypeError when bundle content is "null" or other non-object JSON. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rekmarks
approved these changes
Jan 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace
@endo/bundle-sourcewith Vite for vat bundling and@endo/import-bundlewith directCompartment.evaluate()for bundle loading.Motivation
The Endo bundling tools have complex dependency chains and produce large bundles. Vite provides a simpler, faster bundling pipeline with better tree-shaking and smaller output.
Changes
Bundling
bundleVatfunction using Vite'sbuild()API with IIFE outputbundle,watch) updated to use Vite bundlerBundle Loading
loadBundle()evaluates bundles viaCompartment.evaluate()moduleFormat,code,exports, andmodulespropertiesDependencies
@endo/bundle-source(except from@metamask/kernel-shims),@endo/import-bundlevite,acorn(for comment stripping)Bundle Format
{ "moduleFormat": "iife", "code": "var __vatExports__ = (function() { ... })();", "exports": ["buildRootObject"], "modules": { "./foo.js": { "renderedExports": [...], "removedExports": [...] } } }Migration
Existing bundles using
endoZipBase64format are no longer supported. Regenerate bundles usingocap bundle <entry>.Closes: #742
Note
High Risk
High risk because it changes the vat bundle format and the security-sensitive code-loading path (replacing
@endo/import-bundlewithCompartment.evaluate()), which could break existing bundles or alter sandboxing behavior.Overview
Replaces Endo-based vat bundling/loading with a Vite + IIFE pipeline. The CLI
bundle/watchflow now uses a new Vite-basedbundleVatbuilder (plus Rollup plugins to strip comments and capture export metadata) and drops@endo/bundle-source/@endo/initin favor of@metamask/kernel-shims/endoify.Updates the kernel to consume the new bundle format.
VatSupervisornow fetches bundle content as text and loads it via a newloadBundle()helper that validatesmoduleFormat: "iife"and evaluates the bundle in a SESCompartment, andkernel-utilsadds aVatBundletype +isVatBundleguard with tests. Integration/fixture bundles and tests are updated accordingly, and@endo/import-bundleis removed fromocap-kerneldependencies.Written by Cursor Bugbot for commit 4d3ef97. This will update automatically on new commits. Configure here.